Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves and merges db vs xmp data when displaying an image. #73

Merged
merged 2 commits into from
May 11, 2018

Conversation

IonoclastBrigham
Copy link
Collaborator

These are purely UI/data loading considerations. It is wholly non-destructive, and deciding which tags/rating to save for the image is left to the user.

Strategy was to first extract the rating loading code from the tag loading, then simplify db vs xmp codepaths to just set values which can be compared, and finally decide what to display when we know what's up.

I feel like this code is pretty clumsy and suboptimal, but it seems to behave correctly. We may want to simplify and clean it up further, before merging to master.

Testing Note: Took me a while to realize that the XMP editor was clever enough to also update the database. This made it look like my code changes weren't working, because it still always just displayed what was set in the embedded tags. In order to force an image out of sync, I had to load up a SQLite editor and hack the appropriate rows manually.

Copy link
Owner

@lehitoskin lehitoskin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In these changes, we set-box! when there's embed-support?, but we forget to do it when for the else clause. Line 817 is where a (set-box! image-xmp empty) should go.

@lehitoskin lehitoskin merged commit 6a46b7c into lehitoskin:master May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants